Keep Command Center open during multicursor command execution#4437
Conversation
…3995) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Committed the fix (it was missing from the branch — only the "Initial plan" commit existed). The |
BayuAri
left a comment
There was a problem hiding this comment.
Test Result:
- Issue A: [iOS] Command Center is abruptly slid in when the app tries to display focus in view during multiple selection is active >> Command Center is not moving as expected. There is no layout nor scroll shift issue
- Issue B: Command Center is auto dismissed in 6-8 secs after tapping text formatting button
@copilot
Please fix issue B
…ommand (#3995 Issue B) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
…es formatting (#3995 Issue B) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Fixed Issue B in 5c7df6f's follow-ups. Root cause: my Issue A fix combined the guard into Fix: nested the Coverage: a unit test (no |
…ate in sync for multicursor (#3995) Switch bold/italic/underline/strikethrough commands from formatSelection (document.execCommand) to formatWithTag, which edits the thought value directly. execCommand is a no-op on iOS WebKit when no editable is focused (the multiselect/Command Center flow), so formatting was silently dropped. Keep the toolbar command state in sync after the value-based edit: refresh commandStateStore deterministically in formatWithTag; for a collapsed caret, updateCommandState reads the fresh cursor-thought value instead of the stale post-edit DOM; and skip the cursor-change updateCommandState while a multicursor command is executing so the restored (possibly unselected) cursor does not clobber the selection's state.
#3995) When a multicursor formatting command (bold/italic/underline/strikethrough/code) completes while the selected thoughts are scrolled offscreen (e.g. when the Command Center is onscreen), snap the topmost selected thought into view so the formatted thoughts remain visible. Reuses scrollCursorIntoView, which no-ops when the thought is already in view, so it does not snap when the selection is in focus.
…s focused (#3995) The collapsed-caret branch added for the multicursor formatting fix reads the cursor thought's value, but a focused note is a separate thought from the cursor thought, so its color/formatting was no longer detected. This broke toggling a note's color off (the swatch re-applied instead of removing). Read selection.html() when noteFocus is set, restoring the pre-existing note behavior while preserving the collapsed-caret fix for regular thoughts.
|
Applied the Why: The TDD workflow un-skips the changed Puppeteer test ( This is expected and not a gap in the fix. The underlying defect (#3995) is iOS-WebKit-only: The real regression guard for this fix is the unit test All other checks (Lint, Test, Puppeteer, BrowserStack) are green. |
|
@copilot Test Result:
Please fix Issue G |
…llCursorIntoView (#3995 Issue G) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Fixed Issue G in 5a44331. Root cause: Fix: the area covered by the Command Center is now treated as a bottom obstruction (the max of the navbar height and the Command Center height), used in both the Coverage: a unit regression test ( |
BayuAri
left a comment
There was a problem hiding this comment.
Test case:
-
Issue A: [iOS] Command Center is abruptly slid in when the app tries to display focus in view during multiple selection is active >> Command Center is not moving as expected. There is no layout nor scroll shift issue
-
Issue B: Command Center is auto dismissed in 6-8 secs after tapping text formatting button
-
Issue C: Thought Selection alert is prompted after applying Text Formatting
-
Issue D: Text formatting is not applied on the selected thoughts
-
Issue E: Keyboard flashes when applying Text Formatting
-
Issue F: Focus is not auto scrolled down making the thought above visible when the selected thought is at the top of the focus
-
Issue G: Focus is not auto scrolled up making the thought above visible when the selected thought is at the bottom of the focus
-
Issue H: Thought is scrolled up and down when applying text formatting to Select ALL
Step to Reproduce
- Create 15 thoughts
- Select All the thought
- Tap Bold or any other Text Formatting buttons
Current behavior
Thought jumps up and down
Expected behavior
Text formatting is applied without triggering auto scroll
|
@copilot Issue H is confirmed fixed but Issue F is back on mobile Safari, the thought is jump and down then hidden under the keyboard |
…ion, not just Select All (#3995 Issue F) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Fixed the Issue F regression on mobile Safari in c9e5f60. Root cause: the cursor-follow autoscroll ( Fix: generalized the guard so cursor-follow autoscroll stands down for any active multiselection, not just Select All. The single intended scroll (snapping the topmost selected thought clear of the Command Center) is still performed once by the formatting command's |
|
|
BayuAri
left a comment
There was a problem hiding this comment.
Confirmed fixed.
Test case:
- Issue A: [iOS] Command Center is abruptly slid in when the app tries to display focus in view during multiple selection is active >> Command Center is not moving as expected. There is no layout nor scroll shift issue
- Issue B: Command Center is auto dismissed in 6-8 secs after tapping text formatting button
- Issue C: Thought Selection alert is prompted after applying Text Formatting
- Issue D: Text formatting is not applied on the selected thoughts
- Issue E: Keyboard flashes when applying Text Formatting
- Issue F: Focus is not auto scrolled down making the thought above visible when the selected thought is at the top of the focus
- Issue G: Focus is not auto scrolled up making the thought above visible when the selected thought is at the bottom of the focus
- Issue H: Thought is scrolled up and down when applying text formatting to Select ALL
raineorshine
left a comment
There was a problem hiding this comment.
Due to the scrolling regressions that were introduced by the initial fix, this PR has expanded beyond the scope of the issue and has introduced some architectural issues noted below.
If any of the changes could be sliced off and submitted as independent PRs that would make it possible to merge this in chunks. If that's not possible, and these changes truly are all tightly coupled, then we will need an experienced developer to oversee the agent's development and ensure sound architectural decisions.
There was a problem hiding this comment.
I would strongly suggest not creating additional global reactive state. scrollMulticursorIntoViewStore seems like a bad idea. scrollCursorIntoView is supposed to have everything it needs to scroll the cursor into view. Plus, creating a ministore just to pipe the signal through to a useEffect which then calls scrollCursorIntoView also seems bad. This makes the dependencies between the layout, scroll, and formatting very hard to follow.
We need to consider simpler or more architecturally coherent alternatives.
| // Wait past the alert auto-dismiss delay (Alert.tsx clearDelay defaults to 5000ms). Previously a multicursor | ||
| // command on mobile fell through to the desktop alert branch and dispatched the "n thoughts selected" alert, | ||
| // which auto-dismissed and cleared the multicursors, closing the Command Center a few seconds later (#3995 Issue B). | ||
| await new Promise(resolve => setTimeout(resolve, 6000)) |
There was a problem hiding this comment.
This will add 6 seconds to every test run for every developer till the end of time. You need to mock the delays and reduce all timeouts as close to 0 as possible in tests to avoid this.

On iOS, applying a Text Formatting command (e.g. Bold) while a multiselection is active caused two problems: the Command Center abruptly slid in from the top of the screen when thoughts were scrolled out of view (Issue A), and a "thought selected" alert was shown that auto-dismissed after a few seconds and took the Command Center down with it (Issue B).
Root cause
multicursorAlertMiddlewareshows/hides the Command Center on every change to the multicursor count.executeCommandWithMulticursortransiently clears the multicursors (per-thoughtsetCursor) and then restores them, soshowCommandCenterflipstrue → false → truemid-execution. In Chrome these toggles batch into a no-op, but on iOS WebKit — combined with thescrollCursorIntoViewreflow when thoughts are off-screen — the sheet re-animates (Issue A).Gating the whole branch on the combined condition
isTouch && !state.isMulticursorExecutingfixed the flicker but introduced Issue B: during a mobile multicursor commandisMulticursorExecutingistrue, so the condition was false and execution fell through to the desktop alert branch, dispatching theMulticursorActive"n thoughts selected" alert. That alert auto-dismisses after ~5s (Alert.tsxclearDelay) and runsclearMulticursors, which then closed the Command Center.Changes
multicursorAlertMiddleware.ts— nest the!state.isMulticursorExecutingguard insideif (isTouch)(rather than combining it into the condition), so the transient clear/restore during a command no longer touches the Command Center and a mobile multicursor command never falls through to the desktop alert branch. The terminalsetIsMulticursorExecuting({ value: false })dispatch flows through the middleware with the settled multicursor count and reconciles the final state, preserving the close-on-clear behavior for commands like Delete.__tests__/multicursorAlertMiddleware.ts— added regression tests asserting (1)showCommandCenternever goes false across a multicursor formatting command, and (2) noMulticursorActivealert is dispatched on touch during execution.e2e/puppeteer/__tests__/multiselect.ts— added an iPhone-emulation e2e regression test: after tapping Bold on a multiselection, the Command Center stays open past the alert auto-dismiss delay and the formatting (**a**/**b**) is applied to the selected thoughts.Formatting application and scroll-into-view (Issues C–G)
formatWithTag(which edits the thought value directly) instead offormatSelection/document.execCommand, which is a no-op on iOS WebKit when no editable is focused. The toolbar command state is kept in sync after the value-based edit (Issues C, D, E).scrollCursorIntoView.ts— the Command Center sheet (command-menu-panel,maxHeight: 70%) covers the bottom ~70% of the screen during a multiselection. Previously only the navbar was treated as the bottom obstruction, so a selected thought in the covered region was considered "in view" and was never scrolled out from behind the Command Center (Issue G). The area covered by the Command Center is now included in the bottom obstruction (the max of the navbar height and the Command Center height), used in both theisBelowViewportcheck and the downward scroll target, so the topmost selected thought is scrolled above the Command Center (leaving the thought above it visible). When the Command Center is closed/absent, the obstruction falls back to the navbar, so all other callers are unaffected.__tests__/scrollCursorIntoView.ts— added a unit regression test (deterministic scroll geometry in jsdom) asserting that a thought behind the Command Center is scrolled above it; verified to fail on the pre-fix version and pass with the fix.